Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wxGUI: refactoring: Map Display inheriting from wx.Panel #1675

Conversation

lindakarlovska
Copy link
Contributor

@lindakarlovska lindakarlovska commented Jun 22, 2021

Map Display GUI logic is being changed. For purposes of the Single-Window GUI, MapFrameBase was replaced by MapPanelBase. It subsequently influeced all GUI tools which had been build based on MapFrameBase.
All these tools had to be adapted for new wx.Panel based solution. It had to be coded at once so that's the reason why this PR is too long.

There is the list of influenced GUI tools:

  • regular Map Display
  • GCP display (standalone g.gui.gcp)
  • iclass (standalone g.gui.iclass)
  • mapswipe (standalone g.gui.mapswipe)
  • g.gui.image2target
  • g.gui.photo2target
  • g.gui.rdigit
  • g.gui.vdigit
  • d.mon
  • example (standalone g.gui.example)

@lindakarlovska lindakarlovska marked this pull request as draft June 22, 2021 14:23
@petrasovaa
Copy link
Contributor

Some testing:

  • switching map displays doesn't switch tabs
  • renaming tabs in layertree doesn't work (right click on tab - rename)

Comment on lines 515 to 519
# show map display if requested
if show:
self.currentPage.mapdisplay.Show()
self.currentPage.mapdisplay.Refresh()
self.currentPage.mapdisplay.Update()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be probably moved back to layertree, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what the Refresh and Update are for here, just wondering if it's even necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be probably moved back to layertree, no?

Or to CreateNewMapDisplay method? We do not need layertree to get to know the show parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what the Refresh and Update are for here, just wondering if it's even necessary.

You're right.. not needed.

@lindakarlovska
Copy link
Contributor Author

* switching map displays doesn't switch tabs

I corrected this stuff before those intense unsuccessful changes related to closing and then I somehow lost them :/ ... what to say , just sh*t happens

@lindakarlovska lindakarlovska added gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related labels Jun 28, 2021
@petrasovaa
Copy link
Contributor

Check FullScreen methods in MapPanelBase, probably should go to the mixin.

@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Jul 9, 2021

Check FullScreen methods in MapPanelBase, probably should go to the mixin.

Oh, you are right, we need also to check methods inherited from wx.TopLevelWindow

@lindakarlovska
Copy link
Contributor Author

Check FullScreen methods in MapPanelBase, probably should go to the mixin.

What do you think about it now? Shall I move SetPosition, SetSize and SetIcon also to FrameMixin?

def SetFocus(self):
self.GetParent().SetFocus()

def OnFullScreen(self, toolbars, auimgr, event):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be def ShowFullScreen(self, show) to resemble the actual TopLevelWindow method and inside you would hide/show the toolbars and I think also statusbar. Method IsFullScreen should be defined outside of this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coded :-)

@petrasovaa
Copy link
Contributor

Check FullScreen methods in MapPanelBase, probably should go to the mixin.

What do you think about it now? Shall I move SetPosition, SetSize and SetIcon also to FrameMixin?

I don't think that's needed.

@lindakarlovska
Copy link
Contributor Author

I am not sure how to deal with DMonGrassInterface. I think after merging #1689, we could remove it. What do you think @petrasovaa?
This interface is also used in v.digit and r.digit.

@petrasovaa
Copy link
Contributor

I am not sure how to deal with DMonGrassInterface. I think after merging #1689, we could remove it. What do you think @petrasovaa?
This interface is also used in v.digit and r.digit.

Why? I didn't review the latest changes, so I am probably missing something.

@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Jul 12, 2021

I am not sure how to deal with DMonGrassInterface. I think after merging #1689, we could remove it. What do you think @petrasovaa?
This interface is also used in v.digit and r.digit.

Why? I didn't review the latest changes, so I am probably missing something.

We actually inherit from MapPanelBase because all these panels - d.mon, r.digit, v.vidit inherit from mapdisp/MapPanel . And methods dealing with toolbars and statusbar and probably all other methods left in DMonGrassInterface could be (after #1689) taken from the MapPanelBase class.

@petrasovaa
Copy link
Contributor

Check changes to g.gui.iclass.py, this looks like a mistake.

@lindakarlovska
Copy link
Contributor Author

What I have also noticed when launching these four lines are shown:

(wxgui.py:114279): Gtk-CRITICAL **: 10:46:16.811: gtk_widget_set_size_request: assertion 'width >= -1' failed

(wxgui.py:114279): Gtk-CRITICAL **: 10:46:16.909: gtk_widget_set_size_request: assertion 'width >= -1' failed

(wxgui.py:114279): Gtk-CRITICAL **: 10:46:16.909: gtk_widget_set_size_request: assertion 'width >= -1' failed

(wxgui.py:114279): Gtk-CRITICAL **: 10:46:17.009: gtk_widget_set_size_request: assertion 'width >= -1' failed

And they are then shown whenever I add a new mapdisplay.
Do you have an idea how to suppress them?

@petrasovaa
Copy link
Contributor

What I have also noticed when launching these four lines are shown:

(wxgui.py:114279): Gtk-CRITICAL **: 10:46:16.811: gtk_widget_set_size_request: assertion 'width >= -1' failed

And they are then shown whenever I add a new mapdisplay.
Do you have an idea how to suppress them?

That's a pain and not easy to suppress or figure out what causes them. Are these new, i.e. caused by this PR? If yes, it may be worth finding out the gui component causing it.

@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Jul 30, 2021

@petrasova I have scanned gui if there is any SingleMapFrame or DoubleMapFrame stuff remaining and I have found "SingleMapFrame" in doc/gui/wxpython/example.py.. shell I deal with that?

@petrasovaa
Copy link
Contributor

@petrasova I have scanned gui if there is any SingleMapFrame or DoubleMapFrame stuff remaining and I have found "SingleMapFrame" in doc/gui/wxpython/example.py.. shell I deal with that?

Good catch, yes please look at it.

@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Jul 30, 2021

@petrasovaa It looks like we did forget to change g.gui.mapswipe with the thing of removing StandaloneGrassInterface. Sorry, I have not noticed before, there were lots of files in #1729 ... I can address it here. What do you think?

@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Jul 30, 2021

@petrasovaa It looks like we did forget to change g.gui.mapswipe with the thing of removing StandaloneGrassInterface. Sorry, I have not noticed before, there were lots of files in #1729 ... I can address it here. What do you think?

Actually both - mapswipe/frame.py as well as mapswipe/g.gui.mapswipe.py, if I look right. We can create another small PR for it as the correction of #1729. The same thing is needed for ExampleMapPanel where on the top of it we also forgot about statusbars - so we could merge these final touches to one smaller PR.

@lindakarlovska lindakarlovska force-pushed the wxGUI-refactoring-Map-Display-inherits-from-wxPanel branch from 60ab28d to 4e6847c Compare December 8, 2021 08:49
@lindakarlovska
Copy link
Contributor Author

@petrasovaa I have tested it, and it looks good. I think it is ready to merge.

@petrasovaa petrasovaa merged commit 2f94d78 into OSGeo:main Dec 16, 2021
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
Map Display GUI logic is being changed. For purposes of the Single-Window GUI, MapFrameBase was replaced by MapPanelBase. It subsequently influeced all GUI tools which had been build based on MapFrameBase.
All these tools had to be adapted for new wx.Panel based solution.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
Map Display GUI logic is being changed. For purposes of the Single-Window GUI, MapFrameBase was replaced by MapPanelBase. It subsequently influeced all GUI tools which had been build based on MapFrameBase.
All these tools had to be adapted for new wx.Panel based solution.
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
Map Display GUI logic is being changed. For purposes of the Single-Window GUI, MapFrameBase was replaced by MapPanelBase. It subsequently influeced all GUI tools which had been build based on MapFrameBase.
All these tools had to be adapted for new wx.Panel based solution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants